Add basic support for LeetCode #114
Conversation
- accept `json` as an extra arg - deal with unknown HTTP response status codes from any of services
### supported - get-problem - no options - get-service - no options - login-service, - check only - submit-code - guess-language-id ### not supported - get-contest
|
@kmyk 追加でいくつかNotesです
CIについて
お手すきの際にご確認いただければ幸いです！ |
Notes
了解です。reCAPTCHA の突破を頑張るべきではないので、ユーザには api-client/onlinejudge/type.py Line 31 in a58d7d8
分かりました。ひとまずはこのままマージして、整形をどうするかは後で考えましょう。 CICI の問題の原因はどちらもこのプルリクエストではなかったので、こちらで修正しておきました。rebase してもらえば解決すると思います。
ちょうど発生している問題 (online-judge-tools/verification-helper#335) の影響のようです。ひとまず回避策を入れました (#115)
いいえ、そのままで大丈夫です。Python 3.5 はすでに EOL を迎えている (https://www.python.org/dev/peps/pep-0478/) ので、破壊的変更にはなりますが対応を切ってしまって大丈夫でしょう (#116) |
|
@kmyk ご対応ありがとうございます！ |
|
メソッド追加のコミットを追加しました |
There was a problem hiding this comment.
基本はよさそうです。
しかしいくつか修正や確認しておきたい点があります。スクレイピングツール特有の面倒な議論なのでしんどいのですが、対応お願いします 🙇
また、サンプルダウンロード機能だけでよいのでテストの追加もお願いします。他のファイル (例: https://github.com/online-judge-tools/api-client/blob/master/tests/get_problem_yukicoder.py) をコピペしてすこし修正するだけでできると思います。テストの追加はつい省略したくなりますが、テストがないと「LeetCode の仕様変更で動かなくなり、しかしユーザは誰も報告してくれず、数ヶ月間ずっと動かないままでみんな困っていた」みたいな状況が発生するためです (複数回経験済み)。
CI は走ってるようなので rebase はなしで大丈夫です。
| service_url = self.get_url() | ||
| session.headers.update({ | ||
| 'Origin': service_url, | ||
| 'Referer': service_url, | ||
| 'Content-type': 'application/json', | ||
| }) | ||
|
|
||
| # get csrf token from cookies and set it to header as well | ||
| for cookie in session.cookies: | ||
| if cookie.domain == 'leetcode.com' and cookie.name == 'csrftoken': | ||
| if cookie.value is not None: | ||
| session.headers.update({ | ||
| 'X-CSRFToken': cookie.value, | ||
| }) | ||
| break |
There was a problem hiding this comment.
optional: session は連続して利用するのが期待されるものなので session.headers を書き換えるのはあまりきれいではありません。たとえば以下のように実行すると AtCoder 社のサーバに LeetCode 向けの CSRF token などが送られてしまってセキュリティ的に不適切です。
session = requests.Session()
LeetCodeProblem(...).download_sample_cases(session=session)
AtCoderProblem(...).download_sample_cases(session=session)utils.request(..., headers={'X-CSRFToken': ..., ...}, session=session) のようにすればその headers が追加されて送られます。
実際のところこの挙動が問題になることはないだろうので、ひとまずコメントに説明を書いておくだけでもかまいません。
There was a problem hiding this comment.
たとえば以下のように実行すると AtCoder 社のサーバに LeetCode 向けの CSRF token などが送られてしまってセキュリティ的に不適切です。
これ気が回せていませんでした。そのとおりだと思います。
リクエスト時にheadersを渡す方向で考え直してみます。
| content_html = json_resp['data']['question']['content'] | ||
| if content_html is None: | ||
| logger.warning("This problem seems to be locked: need premium?") | ||
| return [] |
There was a problem hiding this comment.
実は SampleParserError というのが用意されています。「0 個のサンプルがあった」ではなく「サンプルの取得に失敗した」だと思うので、こちらを使ってほしいです。表示されるエラーメッセージがすこし分かりやすいものになるはずです。
api-client/onlinejudge/type.py
Line 106 in a58d7d8
| if idx < len(children): | ||
| output_data = children[idx].strip() | ||
|
|
||
| if input_data and output_data: |
There was a problem hiding this comment.
input_data and output_data が偽な場合になにか警告を表示してほしいです。ほとんど常に成功するのなら、偽な場合をエラーにしてしまってもよいかもしれません。
「実は5個あるサンプルのうちの1個だけ取得に失敗していて、そのことに気付かなかった」などの事態があるとよくないためです。
There was a problem hiding this comment.
なるほどです。
こちら如何せん人が読む問題文中から抽出しているため、LeetCode側の不備でブレやエラーがありそうだなと思い、このような実装にしていました。実際に第1問が
Input: nums = [2,7,11,15], target = 9
Output: [0,1]
Output: Because nums[0] + nums[1] == 9, we return [0, 1].
というふうにOutputが2つ書かれていて、2つめはOutputではなく説明になっていたりします。
（第2問では
Input: l1 = [2,4,3], l2 = [5,6,4]
Output: [7,0,8]
Explanation: 342 + 465 = 807.
のようになっています）
ほとんどの場合、テストケースは
Input: ...
Output: ...
Explanation: ...
もしくは
Input: ...
Output: ...
と書かれているかと思うので、これを想定して、そうでない場合にErrorとするのもありかなと思うのですが、その場合、いきなり第1問のサンプルケースが取得できないこともあり、判断に迷っています。
There was a problem hiding this comment.
第1問からエラーになるのはつらいというのはそうですね。どういうフォーマットがどれくらいの割合なのかは私は分からない (LeetCode にはほとんど参加していない) ので、警告の表示だけかエラーで落とすかの判断は @usk83 さんに任せます。
| try: | ||
| resp = utils.request('POST', f'https://leetcode.com/problems/{self.title_slug}/submit/', session=session, json=json_body) | ||
| except requests.exceptions.HTTPError as e: | ||
| if e.response.status_code != 429: |
There was a problem hiding this comment.
Too Many Requests はどの程度の頻度で起こりますか？ web 上から手動で提出した場合でも提出頻度が原因のエラーは発生することはありますか？ これが起こるのは api-client 側が原因なのか使うユーザが原因なのかどちらが多いですか？
「oj-api から提出だと普通に使っていてもたまに Too Many Requests で弾かれて不便である」「web からの提出ではそのようなことはない」という状況なら retry は実装するべきでしょう。そうでないなら retry はあまりしたくありません。特に「もしユーザが大量に連続提出を試みた場合に Too Many Requests で弾かれる」ということなら retry をすべきではありません。ツールからの自動提出というのは一般にサービス運営側からはあまり歓迎されないものですし、うっかり負荷をかけすぎてジャッジが詰まりコンテストが unrated になったりすると面倒なことになります。
There was a problem hiding this comment.
特に「もしユーザが大量に連続提出を試みた場合に Too Many Requests で弾かれる」ということなら retry をすべきではありません。ツールからの自動提出というのは一般にサービス運営側からはあまり歓迎されないものですし、うっかり負荷をかけすぎてジャッジが詰まりコンテストが unrated になったりすると面倒なことになります。
なるほどです。
Too Many Requestsのエラーですが、実際にLeetCode上でやってみるとまずSubmitして結果が返ってきてから（ここまで1-3秒）、すぐに（1-3秒程度）再度Submitするとこのエラーになり、自分の場合はたまに遭遇するくらいな感じです。
アカウントがFreeかPremiumかにもよると認識していて、Freeアカウントの場合は上記のような数秒の待機時間があるので、うっかりミスにすぐ気づいて再投稿したりすると本ツールの使用の有無にかかわらずユーザの問題で発生します。
自分が普通にサイト上でやっているときは、このエラーになると通るまで数回連打するようなことがあるのでリトライしてくれたら楽だなと考えましたが、
もしユーザが大量に連続提出を試みた場合に
を考えると無理にretryはしないほうがいいかなという考えが少し強くなりました。
retryはしない方向でいかがでしょうか？
| if json_resp['state'] == 'SUCCESS': | ||
| break | ||
| logger.warning('Waiting for the result of your submission(id: %s)', submission_id) | ||
| time.sleep(1 / 3) |
There was a problem hiding this comment.
nit: もうすこし長めに待ちたいです。とりあえず 1 秒待てば許されるはずなのでとりあえず 1 秒にしておきたい。長めに待機を挟むとたいていのユーザからは不満が出るのですが、online-judge-tools のような微妙にグレーなツールでは「サーバに無闇に負荷をかけることはありません」と言えることの方が重要です。
| time.sleep(1 / 3) | |
| time.sleep(1) |
There was a problem hiding this comment.
こちら実際のWebサイトのリクエストをみて、肌感同じくらいな感じに設定しました（実際にsubmitしたあと結果をpollingする実装になっている模様）
伸ばしたほうがいいですか？
There was a problem hiding this comment.
こちら実際のWebサイトのリクエストをみて、肌感同じくらいな感じに設定しました（実際にsubmitしたあと結果をpollingする実装になっている模様）
そういうことなら 1/3 秒でも大丈夫だと思います。ただし、このことをコメントとして書いておいてほしいです。
request_test.py
Outdated
| @@ -0,0 +1,70 @@ | |||
| import json | |||
There was a problem hiding this comment.
完全に油断しました......
これ最新のcommitを編集してforce pushしちゃってもいいでしょうか？
There was a problem hiding this comment.
はい。force push でも削除する commit の追加でもどちらでも大丈夫です
There was a problem hiding this comment.
このために作成したアカウントなので大きな問題はないのですが、セッションIDとか思い切り載せちゃってましたね。まずいまずい😲
ご丁寧にレビューいただきありがたい限りです:bow:
ですよね。Pythonでテストを書いたことがなかったのでついサボりました。 |
To enable logging in to LeetCode using https://github.com/online-judge-tools/oj/
usk83
force-pushed
the
feature/add-leetcode-support
branch
from
d6a38c2 to
b4fde5f
Compare
related to #21
What this PR have done
guess-language-idfeatureWhat this PR does not include